-
Notifications
You must be signed in to change notification settings - Fork 693
Slashable vote data cleanup #1697
Slashable vote data cleanup #1697
Conversation
i also found a bug in the test code so good thing you took a second look! and while normally we should add a test for the broken test case, i want to discuss updating the BLS spec before making any further changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hwwhww 👍 from me structurally, relying on you to review correctness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry! Some more nitpicks of the previous PR.
@hwwhww i made those updates :) the related tests pass locally -- trying to sort why the CI failed... seems unrelated to these edits... |
@ralexstokes thanks! 👍 |
The previous implementation was using the `domain_type` as the `domain` when it should in fact be the value calculated by `get_domain`.
This change is more in line with how this functionality will ultimately be used so we want to route our query through the `BeaconState`.
c8a2bbe
to
569849c
Compare
What was wrong?
Fixes #1696.
How was it fixed?
Implemented the suggested changes from the review here: #1658 (review)
Cute Animal Picture